-
-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Don’t pass response to AutoRouter missing handler #254
Conversation
@istarkov - this is great, and a good catch. I'll want to add a test for this before release, so hang tight (unless you want to add that to the PR) |
I’ve created a simple test to verify that the first argument of the missing function is the request. |
I also fixed the StatusError imports because they were not resolved properly when the package was imported, causing all properties of StatusError to have the any type. See the “Should give TypeScript error” comment for more details. Example: import { Router, type IRequest } from 'itty-router';
type CFArgs = [Env, ExecutionContext];
const router = Router<IRequest, CFArgs>({
catch: (error) => {
// eslint-disable-next-line no-console
console.error(error);
error.status satisfies number;
error.status satisfies string; // <-- Should give typescript error
return new Response(error.message, {
status: error.status,
});
},
}); These errors will be visible if "moduleResolution": "bundler" is set in tsconfig.json under compilerOptions. Alternatively, with the new module: "preserve" option in TypeScript 5.4, the bundler option will be implied. Additionally, the baseUrl has been removed. So, I’ve also updated the tsconfig.json. Splitted PR on 2 commits instead of 2 PR's. |
Also found
2 ways to fix, give an unknown type as in modern ts, or fomat error |
Taking a dive on this today - we've had a bit of a hurricane rip through this week :D |
Interesting test had no issues with [email protected] but fails on [email protected]. Investigating |
Fixed. |
This is great, thanks for the catch! |
Description
The missing handler is a RequestHandler, and the response should not be passed to it.
Probaly a breaking change for non typescript users.
(Found that prettier is not applied to the codebase, or some prettier defaults is not set see
yarn prettier
)Related Issue
Link to the related issue:
Type of Change (select one and follow subtasks)